Skip to content

Conversation

@16bit-ykiko
Copy link
Contributor

@16bit-ykiko 16bit-ykiko commented Sep 10, 2024

This commit adds code completion for C++20 keywords, fix #107868.

  1. complete concept in template context

    • template<typename T> conce^ -> concept
    • conce^
  2. complete requires

    • constraints in template context: template<typename T> requi^ -> requires
    • requires expression: int x = requ^ -> requires (parameters) { requirements }
    • nested requirement: requires { requ^ } -> requires expression ;
  3. complete coroutine keywords

    • co_await^ in expression: co_aw^ -> co_await expression;
    • co_yield in function body: co_yi^ -> co_yield expression;
    • co_return in function body: co_re^ -> co_return expression;
  4. specifiers: char8_t, consteval, constinit

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Sep 10, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 10, 2024

@llvm/pr-subscribers-clang

Author: ykiko (16bit-ykiko)

Changes

This commit adds code completion for C++20 keywords.

  1. complete concept in template context

    • template&lt;typename T&gt; conce^ -> concept
    • conce^
  2. complete requires

    • constraints in template context: template&lt;typename T&gt; requi^ -> requires
    • requires expression: int x = requ^ -> requires (parameters) { requirements }
    • nested requirement: requires { requ^ } -> requires expression ;
  3. complete coroutine keywords

    • co_await^ in expression: co_aw^ -> co_await expression;
    • co_yield in function body: co_yi^ -> co_yield expression;
    • co_return in function body: co_re^ -> co_return expression;
  4. specifiers: char8_t, consteval, constinit


Full diff: https://github.com/llvm/llvm-project/pull/107982.diff

1 Files Affected:

  • (modified) clang/lib/Sema/SemaCodeComplete.cpp (+69)
diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp
index 88d4732c7d5c6a..4647d65430b8c3 100644
--- a/clang/lib/Sema/SemaCodeComplete.cpp
+++ b/clang/lib/Sema/SemaCodeComplete.cpp
@@ -1837,6 +1837,10 @@ static void AddTypeSpecifierResults(const LangOptions &LangOpts,
       Builder.AddChunk(CodeCompletionString::CK_RightParen);
       Results.AddResult(Result(Builder.TakeString()));
     }
+
+    if(LangOpts.CPlusPlus20){
+      Results.AddResult(Result("char8_t", CCP_Type));
+    }
   } else
     Results.AddResult(Result("__auto_type", CCP_Type));
 
@@ -1889,6 +1893,10 @@ AddStorageSpecifiers(SemaCodeCompletion::ParserCompletionContext CCC,
     Results.AddResult(Result("constexpr"));
     Results.AddResult(Result("thread_local"));
   }
+
+  if (LangOpts.CPlusPlus20) {
+    Results.AddResult(Result("constinit"));
+  }
 }
 
 static void
@@ -1912,6 +1920,9 @@ AddFunctionSpecifiers(SemaCodeCompletion::ParserCompletionContext CCC,
   case SemaCodeCompletion::PCC_Template:
     if (LangOpts.CPlusPlus || LangOpts.C99)
       Results.AddResult(Result("inline"));
+
+    if (LangOpts.CPlusPlus20)
+      Results.AddResult(Result("consteval"));
     break;
 
   case SemaCodeCompletion::PCC_ObjCInstanceVariableList:
@@ -2254,6 +2265,10 @@ AddOrdinaryNameResults(SemaCodeCompletion::ParserCompletionContext CCC,
     [[fallthrough]];
 
   case SemaCodeCompletion::PCC_Template:
+    if (SemaRef.getLangOpts().CPlusPlus20 && CCC == SemaCodeCompletion::PCC_Template)
+        Results.AddResult(Result("concept", CCP_Keyword));
+    [[fallthrough]];
+
   case SemaCodeCompletion::PCC_MemberTemplate:
     if (SemaRef.getLangOpts().CPlusPlus && Results.includeCodePatterns()) {
       // template < parameters >
@@ -2266,6 +2281,11 @@ AddOrdinaryNameResults(SemaCodeCompletion::ParserCompletionContext CCC,
       Results.AddResult(Result("template", CodeCompletionResult::RK_Keyword));
     }
 
+    if(SemaRef.getLangOpts().CPlusPlus20 && 
+      (CCC == SemaCodeCompletion::PCC_Template || CCC == SemaCodeCompletion::PCC_MemberTemplate)) {
+      Results.AddResult(Result("requires", CCP_Keyword));
+    }
+
     AddStorageSpecifiers(CCC, SemaRef.getLangOpts(), Results);
     AddFunctionSpecifiers(CCC, SemaRef.getLangOpts(), Results);
     break;
@@ -2487,6 +2507,14 @@ AddOrdinaryNameResults(SemaCodeCompletion::ParserCompletionContext CCC,
       Builder.AddPlaceholderChunk("expression");
       Builder.AddChunk(CodeCompletionString::CK_SemiColon);
       Results.AddResult(Result(Builder.TakeString()));
+      // "co_return expression ;" for coroutines(C++20).
+      if (SemaRef.getLangOpts().CPlusPlus20) {
+        Builder.AddTypedTextChunk("co_return");
+        Builder.AddChunk(CodeCompletionString::CK_HorizontalSpace);
+        Builder.AddPlaceholderChunk("expression");
+        Builder.AddChunk(CodeCompletionString::CK_SemiColon);
+        Results.AddResult(Result(Builder.TakeString()));
+      }
       // When boolean, also add 'return true;' and 'return false;'.
       if (ReturnType->isBooleanType()) {
         Builder.AddTypedTextChunk("return true");
@@ -2707,6 +2735,47 @@ AddOrdinaryNameResults(SemaCodeCompletion::ParserCompletionContext CCC,
         Builder.AddChunk(CodeCompletionString::CK_RightParen);
         Results.AddResult(Result(Builder.TakeString()));
       }
+
+      if (SemaRef.getLangOpts().CPlusPlus20) {
+        // co_await expression
+        Builder.AddResultTypeChunk("");
+        Builder.AddTypedTextChunk("co_await");
+        Builder.AddChunk(CodeCompletionString::CK_HorizontalSpace);
+        Builder.AddPlaceholderChunk("expression");
+        Results.AddResult(Result(Builder.TakeString()));
+
+        // co_yield expression
+        Builder.AddResultTypeChunk("");
+        Builder.AddTypedTextChunk("co_yield");
+        Builder.AddChunk(CodeCompletionString::CK_HorizontalSpace);
+        Builder.AddPlaceholderChunk("expression");
+        Results.AddResult(Result(Builder.TakeString()));
+
+        // requires (parameters) { requirements }
+        Builder.AddResultTypeChunk("bool");
+        Builder.AddTypedTextChunk("requires");
+        Builder.AddChunk(CodeCompletionString::CK_HorizontalSpace);
+        Builder.AddChunk(CodeCompletionString::CK_LeftParen);
+        Builder.AddPlaceholderChunk("parameters");
+        Builder.AddChunk(CodeCompletionString::CK_RightParen);
+        Builder.AddChunk(CodeCompletionString::CK_HorizontalSpace);
+        Builder.AddChunk(CodeCompletionString::CK_LeftBrace);
+        Builder.AddChunk(CodeCompletionString::CK_VerticalSpace);
+        Builder.AddPlaceholderChunk("requirements");
+        Builder.AddChunk(CodeCompletionString::CK_VerticalSpace);
+        Builder.AddChunk(CodeCompletionString::CK_RightBrace);
+        Results.AddResult(Result(Builder.TakeString()));
+
+        if(llvm::isa<clang::RequiresExprBodyDecl>(SemaRef.CurContext)){
+          // requires expression ;
+          Builder.AddResultTypeChunk("");
+          Builder.AddTypedTextChunk("requires");
+          Builder.AddChunk(CodeCompletionString::CK_HorizontalSpace);
+          Builder.AddPlaceholderChunk("expression");
+          Builder.AddChunk(CodeCompletionString::CK_SemiColon);
+          Results.AddResult(Result(Builder.TakeString()));
+        }
+      }
     }
 
     if (SemaRef.getLangOpts().ObjC) {

@github-actions
Copy link

github-actions bot commented Sep 10, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@16bit-ykiko
Copy link
Contributor Author

16bit-ykiko commented Sep 10, 2024

Also test with clangd, it works well.

image
image
image
image
image
image
image

Copy link
Collaborator

@shafik shafik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about module, import and export?

@16bit-ykiko
Copy link
Contributor Author

What about module, import and export?

Yes, I miss them and will add soon.

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change needs a release note.
Please add an entry to clang/docs/ReleaseNotes.rst in the section the most adapted to the change, and referencing any Github issue this change fixes. Thanks!

@16bit-ykiko
Copy link
Contributor Author

This change needs a release note. Please add an entry to clang/docs/ReleaseNotes.rst in the section the most adapted to the change, and referencing any Github issue this change fixes. Thanks!

I an not sure which section I should add in clang/docs/ReleaseNotes.rst? There isn't a section related to code completion.

@16bit-ykiko
Copy link
Contributor Author

What about module, import and export?

Added, but I am not sure how to test them. Do I need a actual input module file or just use name started with mod like modulexx to test it.

@zyn0217
Copy link
Contributor

zyn0217 commented Sep 30, 2024

I am not sure which section I should add in clang/docs/ReleaseNotes.rst? There isn't a section related to code completion.

@16bit-ykiko There is an individual file for clang-tools-extras clang-tools-extra/docs/ReleaseNotes.rst which covers changes in clangd. See also #105975.

(AFAICT, for a long time, clangd hasn't had a practice for updating its release notes whenever new features are implemented, but the situation is getting improved recently.)

// export
Results.AddResult(Result("export", CodeCompletionResult::RK_Keyword));

if (SemaRef.CurContext->isTranslationUnit()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we restrict that using getCurrentModule()->Kind @ChuanqiXu9 ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we can't do it here. Since we don't the kind of the current module before we see module declarations.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, exactly.
If we haven't see module;, we should propose module; and export module
If we have seen module; - we should not propose it again
If we have seem export module, we should not propose import

Copy link
Contributor Author

@16bit-ykiko 16bit-ykiko Oct 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I push a new commit and try to make completion for module related keywords more context-sensitive.

And found something strange: https://godbolt.org/z/YM9dhEKKe

module;

export module M;
         ^

If I try to run code completion at ^, only get a compiler error.

<source>:3:1: error: export declaration can only be used within a module purview
    3 | export mo<U+0000>dule M;

The error shouldn't occur, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It only happens during completion so there is probably a bug with that https://godbolt.org/z/7M8EPodoP

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made some modification to ParseExportDeclaration to fix the problem.

@16bit-ykiko
Copy link
Contributor Author

16bit-ykiko commented Nov 12, 2024

@cor3ntin Can we merge this PR? :)

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cor3ntin cor3ntin merged commit ec4c47d into llvm:main Nov 26, 2024
9 checks passed
@16bit-ykiko 16bit-ykiko deleted the complete-cxx20-keywords branch December 24, 2024 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Clang] [CodeComplete] Cannot complete C++20 keywords

7 participants